-
Notifications
You must be signed in to change notification settings - Fork 121
[CIAB] Hide POS tab for CIAB sites #16147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CIAB] Hide POS tab for CIAB sites #16147
Conversation
Generated by 🚫 Danger |
|
|
|
I will resolve conflicts and update release notes according to milestone after a review |
jaclync
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for tagging, the POS code changes look good to me. LegacyPOSTabEligibilityChecker is meant to be deleted soon WOOMOB-869, apology for not removing it sooner to avoid some unnecessary work in this PR. I will work on the removal shortly after this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this update! The POS tab is hidden as expected for CIAB sites and shown for other sites from my testing ✅
The switch from observing site ID to site seems to cause a few issues:
- It takes a bit longer to reload views when switching sites (since site info needs to be fetched again). I suppose this is negligible.
- The Menu tab doesn't seem to load correctly. The store name, address, and sections like Payments, Blaze, Google Ads, Inbox are not displayed for all stores (screencast below). We might want to use
siteto activatehubMenuTabCoordinatorto resolve this.
Simulator.Screen.Recording.-.iPad.mini.A17.Pro.-.2025-09-19.at.11.25.57.mov
| func observeSiteIDForViewControllers() { | ||
| cancellableSiteID = stores.siteID.sink { [weak self] siteID in | ||
| cancellableSiteID = stores.site.sink { [weak self] site in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super nit: should the method and the property be renamed to ~site instead of ~siteID?
|
Another issue with observing Simulator.Screen.Recording.-.iPad.mini.A17.Pro.-.2025-09-19.at.12.20.05.movShould we observe site separately and use it to update POS and Bookings tabs only for simplicity? |
|
@itsmeichigo Thx for spotting the issue. Site is now observed separately in 8b32ea3 |
itsmeichigo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update, all is good now 💯 ![]()

WOOMOB-1363
Description
Hides POS tab for CIAB sites
pointOfSaleenum case toCIABAffectedFeaturesCIABEligibilityCheckerintoPOSTabEligibilityCheckerandLegacyPOSTabEligibilityCheckerPOSTabEligibilityCheckerandLegacyPOSTabEligibilityCheckerto work with injectedsiteinstead ofsiteID. CIAB checker requires site for assessment.MainTabBarControllerobservesstores.siteinstead ofstores.siteIDbecause of the reason above. Also the defaultSite might be blank by the moment of POS eligibility checkers initialization - observing the site resolves the issue.ScreenshotsObjectGraphextractsdefaultSitemock toSite+Mocks.swiftextension for reusability inPreviewHelpers.swiftTesting steps
RELEASE-NOTES.txtif necessary.